Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Make PROC_MACRO_DERIVE_RESOLUTION_FALLBACK a hard error" #107133

Merged
merged 1 commit into from
Jan 22, 2023
Merged

Revert "Make PROC_MACRO_DERIVE_RESOLUTION_FALLBACK a hard error" #107133

merged 1 commit into from
Jan 22, 2023
+238 −49

Conversation

pnkfelix
Copy link
Member

This reverts commit 7d82cad aka PR #84022

I am doing this to buy us some time with respect to issue #106337 w.r.t. the 1.67 release.

@pnkfelix
Revert "Make PROC_MACRO_DERIVE_RESOLUTION_FALLBACK a hard error"
5fa1347 
This reverts commit 7d82cad.

I am doing this to buy us some time with respect to issue rust-lang#106337 w.r.t. the
1.67 release.
@rustbot
Copy link
Collaborator

rustbot commented Jan 20, 2023

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 20, 2023
@pnkfelix pnkfelix removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 20, 2023
@pnkfelix
Copy link
Member Author

beta-nominating, checking with @wesleywiser or @Mark-Simulacrum to see about trying to get this in before the 1.67-beta is uplifted to stable.

@rustbot label: beta-nominated

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 20, 2023
@Mark-Simulacrum
Copy link
Member

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Jan 21, 2023

📌 Commit 5fa1347 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 21, 2023
@Mark-Simulacrum Mark-Simulacrum added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 21, 2023
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Jan 21, 2023
Merged
@petrochenkov
Copy link
Contributor

I don't think that we should revert this, or at least it should be re-landed on the next or next+1 release.
Whatever the solution for #64079 will be, it certainly won't look like what PROC_MACRO_DERIVE_RESOLUTION_FALLBACK does.

cuviper added a commit to cuviper/rust that referenced this pull request Jan 21, 2023
@cuviper
Remove relnote for rust-lang#84022 due to revert rust-lang#107133
43c18e9
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2023
@bors
Auto merge of rust-lang#107164 - Mark-Simulacrum:beta-next, r=Mark-Si…
2276448 
…mulacrum

[beta] backport

*  Don't wf-check non-local RPITs rust-lang#107038
* Revert "Make PROC_MACRO_DERIVE_RESOLUTION_FALLBACK a hard error" rust-lang#107133
* bump bootstrap 1.66.1

r? `@Mark-Simulacrum`
@bors
Copy link
Contributor

bors commented Jan 22, 2023

⌛ Testing commit 5fa1347 with merge 85da15c...

@bors
Copy link
Contributor

bors commented Jan 22, 2023

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 85da15c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 22, 2023
@bors bors merged commit 85da15c into rust-lang:master Jan 22, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 22, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (85da15c): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.7% [0.7%, 0.7%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.4% [-2.4%, -2.4%] 1
All ❌✅ (primary) 0.7% [0.7%, 0.7%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

@pnkfelix
Copy link
Member Author

I don't think that we should revert this, or at least it should be re-landed on the next or next+1 release. Whatever the solution for #64079 will be, it certainly won't look like what PROC_MACRO_DERIVE_RESOLUTION_FALLBACK does.

Obviously I'm in favor of the revert, but I mostly agree with the sentiment of @petrochenkov 's statements. I don't think the lang team has spent very much time at all thinking about #64079, and that there are other points in the solution space that should be considered. I am thinking/hoping we will re-land the hard-error-update sometime in the next 12 to 18 weeks, which I suppose is a slightly longer schedule than what @petrochenkov had proposed.

@pnkfelix pnkfelix mentioned this pull request Jan 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cjgillot cjgillot

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.68.0
Development

Successfully merging this pull request may close these issues.
7 participants
@pnkfelix @rustbot @Mark-Simulacrum @bors @petrochenkov @rust-timer @cjgillot